Skip to content

MODEL-NEMOTRON-H W3 repair: ClaimMamba ignored quantized, and the legacy-alias precedence was INVERTED (#517) - #572

Closed
localai-bot wants to merge 7 commits into
mainfrom
row/MODEL-NEMOTRON-H-W3-FIX
Closed

MODEL-NEMOTRON-H W3 repair: ClaimMamba ignored quantized, and the legacy-alias precedence was INVERTED (#517)#572
localai-bot wants to merge 7 commits into
mainfrom
row/MODEL-NEMOTRON-H-W3-FIX

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

Supersedes #565 (row/MODEL-NEMOTRON-H-W3C @ 3295d0c1a), which a fresh review
returned PASS on with four MINOR/NIT findings and two report-only items.
This branch is that head, with origin/main re-merged (W1's MIXED_PRECISION
resolver having landed as 1bc5ef82c) and the findings repaired.

Issue: #517 — W3 of .agents/specs/nemotron-h-model.md.
Record: spec §5d.

The findings

# Finding Disposition
1 ClaimMamba ignored the quantized flag FIXED
2 Legacy-alias precedence inverted, and the record asserted the opposite of upstream FIXED (behavior + comments + spec §5c)
3 The forward refusal was claimed but never exercised FIXED
4 NemotronHBlockName had zero call sites FIXED (used)
5 docs/FEATURES.md:171 arch count is off by two, pre-existing RECORDED — not fixed, no docs/ authority on this branch
6 Fixture dtype drift is invisible without the checkpoint RECORDED as a residual

1. Every other claimer (ClaimNvfp4, ClaimMoe, ClaimMlp, and
ClaimAttention's fp8_kv) gates on quantized; ClaimFp8 did not. The
released config MINUS quantization_config — the shape a released bf16
NemotronH safetensors checkpoint actually ships, which spec §5b already owes —
parsed without refusal and enumerated 92 tensors that checkpoint does not
have
(23 mamba blocks x 2 projections x 2 companions), first
backbone.layers.0.mixer.in_proj.weight_scale. Nothing consumes the map yet, so
no gated claim was wrong; but an unimplemented arm is refused BY NAME, never
silently mis-enumerated. The 18487-tensor gate is unchanged — the released
checkpoint is quantized and its MTP schedule is {attention, moe}.

2. Upstream is self.n_groups = kwargs.pop("mamba_n_groups") if "mamba_n_groups" in kwargs else self.n_groups
(configuration_nemotron_h.py:145-155), which overwrites an already-populated
dataclass field — so legacy wins for the mamba_* scalars. Re-derived by
RUNNING transformers @ 7d06b1a5, not by reading it:

NemotronHConfig(n_groups=8, mamba_n_groups=4, conv_kernel=4, mamba_d_conv=7)
  -> n_groups=4, conv_kernel=7
NemotronHConfig(layer_types=['mamba','mamba'], hybrid_override_pattern='*-')
  -> ['mamba', 'mamba']

The SCHEDULE pair is genuinely modern-wins and is left alone. The two families
disagree upstream; both polarities are now mirrored and both stated where they
are implemented, each with its own anchor. Worse than the behavior was the
record: the comment and spec §5c asserted "the modern one wins" as upstream's
rule. No released checkpoint ships both spellings of one field, so this was a
mirroring and record defect, not a live one — which is why it needed a test.

Verification

RED first for findings 1 and 2 on the pre-fix tree: 9 failed assertions,
reporting 92 companions with that exact first name, and 8 == 4 / 4 == 7 /
2 == 9 / 128 == 77 / 0.001 == 0.5 / 0.1 == 0.6 / 0.0001 == 0.7.

The review's ten mutations were re-run in full on the repaired tree and all
ten are still RED
, plus four this pass adds (forward returns {};
NemotronHBlockName mislabels kMoe; and a straight regression of each of
findings 1 and 2). Each was applied alone, rebuilt, run, then restored with its
SHA-256 re-verified. Two are worth noting: the mtp* mutation drives the
assertion COUNT up by 1036 while going red, and the NemotronHBlockName
mutation prints assertions: 28 | 28 passed | 0 failed beside 8 failed test
cases — the cases threw, so grep 'assertions:' alone reads it as clean. Full
table in spec §5d.

Local CPU-only host, disk 68G free (85% used) at every measurement:

Arm Result
Release -Werror, full build 0 warnings, 0 errors
test_nemotron_h_scaffold, offline 12/12 cases, 38284/38284 assertions, Status: SUCCESS!
test_nemotron_h_scaffold, VT_NEMOTRON35_SNAPSHOT live 12/12, 39152/39152, Status: SUCCESS!
Debug (-g0, asserts unmasked), offline + live identical
Full ctest -j4 100% tests passed, 0 failed out of 403

W3 baselines were 10/38245 offline and 10/39113 live.

For the operator

One gate is RED and this branch cannot clear it itself.
scripts/agent-preflight.sh --staged is green on everything except:

Committed range vs origin/main:
  FAIL doc-checkpoint range
     ERROR: commit 3981de6a4: changed feature_surface but did not update
     docs/FEATURES.md.

The rule is unconditional and PER-COMMIT: check-doc-checkpoint.py:79
classifies any src/vllm/model_executor/models/ path as feature_surface,
:313 requires docs/FEATURES.md in the same commit, and commits_in_range
walks rev-list --reverse --no-merges origin/main..HEAD — so W3C's own
docs/FEATURES.md edit in 3295d0c1a does not cover a later commit.
ci.yml:343 runs the identical invocation, so CI is red for the same reason.
The checker has no exemption mechanism, deliberately.

This branch was dispatched with docs/ explicitly excluded from its authority,
so it neither took the obvious fix nor weakened the checker. The minimal
truthful edit — a real feature-surface fact this commit changed — is one clause
on the NemotronHForCausalLM row (docs/FEATURES.md:139):

... 18487/18487 released tensors claimed, 0 unaccounted; het-KV shapes match
mamba2_state_shape. A bf16 producer (no quantization_config) claims the
bare weights and none of the 92 FP8/NVFP4 scale companions.

folded into 3981de6a4 (amend, or a squash-merge carrying both).

Finding 5 is a pre-existing docs/FEATURES.md drift this branch deliberately
did not touch and owes an issue: :171 says "27 of the 32 registered
text-generation architectures" while :173 implies 38 − 3 Parakeet − 1
LlamaModel = 34, and tests/vllm/models/test_model_registry.cpp:47 says "34
text archs". W3's +1 increment was correct; the base was already stale, and no
checker validates it.

The row stays INVENTORIED: this makes NemotronH KNOWN, it runs nothing. The
forward (W4), MTP head (W5), e2e token gate (W6) and GGUF arm (W7) all still
refuse by name.

🤖 Generated with Claude Code

mudler added 6 commits August 13, 2026 00:52
…fig + 18487-tensor index, headers only (#517)

FOLLOWING_AGENTS_PROTOCOL

W3 of .agents/specs/nemotron-h-model.md needs a config the parse gate can read
and an index the enumeration gate can be checked against, WITHOUT the 20.1 GiB
checkpoint. Both are projected from the live NAS copy of
nvidia/NVIDIA-Nemotron-3.5-Lightning-30B-A3B-NVFP4 at the pinned revision
29f2d1746d8f41e316523194b19018707749b1b1, reading JSON and safetensors HEADERS
only -- no tensor bytes.

config.json is the released file MINUS exactly two keys,
quantization_config.{config_groups,quantized_layers}: 865 KB of the 1.34 MB
file, 5981 entries each, and the surface W1 (#517 W1) owns. `ignore` (72
entries, 2.4 KB) is DELIBERATELY KEPT -- its `mtp*` wildcard is what makes the
MTP tower unquantized, which the enumeration has to know and would otherwise
have to guess.

index.json projects all 18487 tensors to one entry per (layer index, tensor
suffix) family with dtype and shape; only the ROUTED-EXPERT index is collapsed
to {E}, with `count` recording how many experts share the entry. Layer indices
are PRESERVED on purpose, so a wrong layer-type mapping cannot survive the
fixture. 707 families, sum(count) == 18487, dtypes BF16 485 / F32 6086 /
F8_E4M3 5981 / U8 5935.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…enumerated, KV-shaped (#517)

FOLLOWING_AGENTS_PROTOCOL

W3 of .agents/specs/nemotron-h-model.md. It makes the architecture known; it
runs nothing. The forward REFUSES BY NAME (W4, itself blocked on the Mamba2 SSD
CUDA arm #496 W2) and the GGUF arm REFUSES BY NAME (W7, owed per §5b).

RED FIRST. The gate was written against declared-but-empty implementations and
run before any of it worked, Release -Werror, doctest 2.5.2:

    [doctest] test cases:   10 |    2 passed |  8 failed | 0 skipped
    [doctest] assertions: 1258 | 1186 passed | 72 failed |
    [doctest] Status: FAILURE!

failing in exactly the intended shape -- CHECK( 0 == 52 ) for the schedule
depth, CHECK( {} == {5, 12, 19, 26, 33, 42} ) for the attention indices,
REQUIRE( 0 == 2 ) for the KV groups, "did NOT throw at all!" for the GGUF
refusal, 18487 unclaimed tensors for the enumeration.

ENUMERATION, the hard number: 18487 of 18487 released tensors CLAIMED by a named
consumer. 0 unaccounted, 0 invented, 0 refused. Gated both directions against
the committed headers-only projection of the released index, plus a live case
that re-verifies that projection against the checkpoint itself when
VT_NEMOTRON35_SNAPSHOT names it.

Four things the plan had wrong or unstated, settled against source and disk:

1. layers_block_type is the source of truth -- but vLLM VENDORS a NemotronHConfig
   with the OPPOSITE polarity (hybrid_override_pattern as the ctor arg,
   layers_block_type derived). That class is imported for TYPE ANNOTATION only;
   the runtime object comes from transformers AutoConfig, where num_hidden_layers
   is a property over layers_block_type whose setter discards the checkpoint's
   value. Both spellings parse, the modern one wins.
2. moe_latent_size does NOT need three states: upstream's predicate is
   `getattr(config, "moe_latent_size", None) is not None`, so ABSENT and `null`
   are the same state. std::optional covers both; a real value refuses (§0).
3. The shared detail::ResolveMambaSsmCacheDType is the WRONG reader here and
   silently HALVES the recurrent state. It is keyed on `mamba_ssm_dtype`
   (Qwen3.5's spelling); NemotronH ships `mamba_ssm_cache_dtype`, so it returned
   the CONVOLUTION dtype. Caught by the KV gate as page_size_bytes() 1085440 vs
   an expected 2134016 -- the SSM state at bf16 instead of f32. Not a Qwen bug;
   two families, two config keys. Resolved locally, reason recorded at the call
   site.
4. Upstream's default conv layout is "SD" = (state_len, dim) while ours is
   (dim, state_len). Same bytes; the local convention is kept so the shared
   runner sees one orientation, and the divergence is commented rather than left
   for W4 to rediscover.

KV: two groups carrying REAL per-layer names -- 6 attention layers on
FullAttentionSpec(2 kv heads, head_size 128), 23 mamba layers on MambaSpec
{{6144,3},{64,64,128}} / {bf16,f32}. conv_dim == 6144 is falsified straight off
disk by conv1d.weight BF16 [6144,1,4]. The names are load-bearing:
kv_cache_utils.cpp:979 and kv_cache_interface.cpp:151-158 both multiply a
group's page by layer_names.size(), so a one-element tag under-counts by 23x
and 6x.

SCOPE: no per-module quant algorithm is resolved -- that is W1, not on main. W3
reads four coarse falsifiable keys (quant_method, quant_algo, kv_cache_scheme,
the `mtp*` entry in `ignore`) and derives scale companions STRUCTURALLY; the
18487-tensor gate is what proves the derivation. A non-ModelOpt producer refuses
by name. One piece of debt is recorded in the code: a dense `mlp` block's
quantized companion layout is DERIVED, because no in-scope checkpoint ships one.

nemotron_h.h lives under src/, not include/vllm/: W3 ships nothing on the public
ABI and include/vllm/ is a USER_USAGE_PREFIXES surface (#515).

RECORDS. docs/FEATURES.md gains an honest scaffold row (registering an
architecture IS a feature-surface change, and check-supported-models.py binds the
two), the stale "Inventoried but blocked" entry is removed so the page does not
say two things, and the three registered counts move 37->38 / 31->32.
docs/USAGE.md gains one row in "Architectures that resolve but refuse to run":
check-doc-checkpoint classifies the root CMakeLists.txt as a USER_USAGE file, and
the obligation is real rather than an artifact -- pointing the CLI at a NemotronH
checkpoint now resolves and refuses by name instead of reporting an unsupported
architecture. That is one row past the FEATURES.md-only scope I was given, on the
operator's own reasoning applied to the surface the checker names.

test_model_registry.cpp PINS the registry contents four ways; all four are
reconciled, never weakened: the two size assertions 37->38, both byte-order arch
sequences, kExampleConfigArchitectures, both pinned "Supported architectures:"
refusal strings, and a new explicit NemotronH branch asserting is_hybrid TRUE /
supports_multimodal FALSE instead of letting it fall into the
pure-text-full-attention else-branch. test_model_loader_gguf.cpp's canonical
arch string gains the entry too.

GATES, Release -Werror CPU:
    test_nemotron_h_scaffold      10/10 cases, 38245/38245 assertions, SUCCESS
    with VT_NEMOTRON35_SNAPSHOT   10/10 cases, 39113/39113 assertions, SUCCESS
    Debug arm (asserts unmasked)  identical
    full ctest    100% tests passed, 0 tests failed out of 401
                  (test_voxtral_e2e skipped, no asset)

MUTATION-PROVEN (each applied alone, rebuilt, run, reverted, tree verified clean):
    moe -> attention layer mapping            4 cases /  9 assertions FAILURE
    attention index set shifted by +1         2 cases /  2 assertions FAILURE
    conv_dim without 2*n_groups*state_size    4 cases /  8 assertions FAILURE
    SSM dtype collapsed to the activation     1 case  /  2 assertions FAILURE
    mamba dt_bias left unclaimed              1 case  /  2 assertions FAILURE

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…esolver landed as 1bc5ef8) (#517)

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…acy-alias precedence was INVERTED (#517)

Three review findings on `row/MODEL-NEMOTRON-H-W3C` @ `3295d0c1a` (PR #565).
The verdict was PASS; this is the tightening pass.

1. `ClaimMamba` hard-coded its FP8 companions. Every other claimer
   (`ClaimNvfp4`, `ClaimMoe`, `ClaimMlp`, `ClaimAttention`'s `fp8_kv`) gates on
   `quantized`; `ClaimFp8` did not. The released config MINUS
   `quantization_config` -- the shape a released bf16 NemotronH safetensors
   checkpoint actually ships, which spec 5b already owes -- parsed without
   refusal and enumerated 92 tensors that checkpoint does not have
   (23 mamba blocks x 2 projections x 2 companions), first
   `backbone.layers.0.mixer.in_proj.weight_scale`. Nothing consumes the map
   yet, so no gated claim was wrong -- but an unimplemented arm is refused BY
   NAME, never silently mis-enumerated. `quantized` is now threaded through
   `ClaimFp8`/`ClaimMamba` and both call sites (backbone `quantized`, MTP
   `mtp_quantized`). The released checkpoint is quantized and its MTP schedule
   is {attention, moe}, so the 18487-tensor gate is unchanged.

2. `Get{Int,Double,Bool}Aliased` preferred the MODERN key. Upstream does the
   OPPOSITE for the `mamba_*` scalars: `configuration_nemotron_h.py:145-155` is
   `self.n_groups = kwargs.pop("mamba_n_groups") if "mamba_n_groups" in kwargs
   else self.n_groups`, which OVERWRITES an already-populated dataclass field,
   so LEGACY wins. Re-derived by RUNNING transformers @ 7d06b1a5, not by
   reading it: `NemotronHConfig(n_groups=8, mamba_n_groups=4, conv_kernel=4,
   mamba_d_conv=7)` -> `n_groups=4, conv_kernel=7`. The SCHEDULE pair is
   genuinely modern-wins (`:158-165` consults the pattern only when the list is
   None; `NemotronHConfig(layer_types=['mamba','mamba'],
   hybrid_override_pattern='*-')` -> `['mamba','mamba']`), and is left alone.
   Upstream disagrees with itself between the two families; both polarities are
   now mirrored and both are stated explicitly where they are implemented.
   Worse than the behavior was the RECORD: the comment asserted "the modern key
   wins" as if it were upstream's rule, which is what would mislead the next
   porter. No released checkpoint ships both spellings of one field, so this
   was a mirroring defect, not a live one -- which is exactly why it needed a
   test.

3. "the unported ARMS refuse by name" had one SUBCASE (GGUF).
   `ForwardNemotronHForCausalLM` is an unconditional `VT_CHECK`, which throws
   `std::runtime_error`, so it is directly callable with a stub `LoadedModel`.
   Now asserted, including that the message names W4 and the spec.

4. `NemotronHBlockName` had zero call sites. `BlockFromName` now maps both
   directions through it and builds its refusal's expected-list from the enum,
   so a fifth block kind cannot arrive with a message that still lists four.

RED first for 1 and 2 on the pre-fix tree: 12 cases, 9 assertions failed --
`companions.empty()` reporting 92 with that exact first name, and
`8 == 4`/`4 == 7`/`2 == 9`/`128 == 77`/`true`/`0.001 == 0.5`/`0.1 == 0.6`/
`0.0001 == 0.7`. GREEN after: 12 cases / 38284 assertions / 0 failed,
`Status: SUCCESS!` (baseline was 10 / 38245).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…ep, and two residuals (#517)

Records what neither the code nor git carries: which of the fresh review's
findings were fixed and how, the two report-only items, and the full mutation
proof re-run on the repaired tree.

Also CORRECTS 5c. Its reconnaissance item 1 said both spellings are accepted
"the modern one winning" as an unqualified rule. That is true of the SCHEDULE
pair only. For the `mamba_*` scalars upstream is the OPPOSITE
(configuration_nemotron_h.py:145-155 overwrites the already-populated field),
re-derived by RUNNING transformers @ 7d06b1a5. A record that states the
opposite of upstream is the part that misleads the next porter, so the
generalization is scoped and 5d carries the evidence.

Residuals recorded, NOT fixed here:
- docs/FEATURES.md:171 says "27 of the 32 registered text-generation
  architectures" while :173 implies 38-3-1 = 34 and test_model_registry.cpp:47
  says "34 text archs". W3's +1 was right; the BASE was already stale and no
  checker validates it. This branch has no docs/ authority, and repairing a
  pre-existing doc drift inside a scoped repair would hide it.
- Fixture DTYPE drift is invisible offline by design: names and shapes are
  pinned in the committed projection, dtypes only by the live case, and CI has
  no checkpoint. Named so W4/W6 do not rediscover it.

5b's safetensors-bf16 row now says its enumeration is correct.

Gate evidence, disk 68G free (85%) throughout: Release -Werror full build 0
warnings; scaffold 12/12 cases 38284/38284 offline and 12/12 39152/39152 live;
Debug (-g0, asserts unmasked) identical on both; full ctest 100% passed, 0
failed out of 403.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…elf -- doc-checkpoint needs docs/ authority (#517)

`scripts/agent-preflight.sh --staged` is green on every gate except
`doc-checkpoint range`, which fails on `3981de6a4`: it touched
`src/vllm/model_executor/models/` without touching `docs/FEATURES.md`.

The rule is unconditional and PER-COMMIT --
`check-doc-checkpoint.py:79` classifies any `src/vllm/model_executor/models/`
path as `feature_surface`, `:313` requires `docs/FEATURES.md` in the SAME
commit, and `commits_in_range` walks `rev-list --reverse --no-merges
origin/main..HEAD`, so W3C's own `docs/FEATURES.md` edit in `3295d0c1a` does not
cover a later commit. `ci.yml:343` runs the identical invocation on the PR, so
the PR is red for the same reason. The checker has no exemption mechanism, by
design.

This repair branch was dispatched with `docs/` explicitly excluded from its
authority, so it does not take the obvious fix and does not weaken the checker
to go green. Recorded in the spec with the exact minimal edit the operator can
fold into `3981de6a4`: one truthful clause on the `NemotronHForCausalLM` row
naming the bf16 enumeration this commit corrected. It disturbs neither W3's
existing `docs/` edits nor the separate finding-5 count drift.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…ce (#517)

`72e661ae4` is a SPEC-DSPARK (#442) records commit: .agents/benchmark-record.md,
.agents/specs/dspark-spec-decode.md, docs/BENCHMARKS.md, docs/STATUS.md. It
touches no file this row's gates compile or execute, so the gate evidence
recorded in spec 5d stands unchanged rather than being re-asserted from a
rebuild it cannot move. That claim is mechanically verified in the commit that
follows, not eyeballed.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
@localai-bot
localai-bot deleted the row/MODEL-NEMOTRON-H-W3-FIX branch August 13, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants